Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow specify property value without lambda in ExecuteUpdate #29139

Merged
merged 1 commit into from
Sep 20, 2022
Merged

Allow specify property value without lambda in ExecuteUpdate #29139

merged 1 commit into from
Sep 20, 2022
+229 −78

Conversation

roji
Copy link
Member

@roji roji commented Sep 19, 2022 •
edited by ajcvickers
Loading

Fixes #28968

Description

This is a non-breaking tweak to a new API introduced in EF7. It allows a simpler form to be used for ExecuteUpdate. Specifcally,

.SetProperty(p => p.Name, "Shay")

can be used instead of

.SetProperty(p => p.Name, _ => "Shay")

There may be many calls to SetProperty and setting to a value that does not depend on any other values in the entity is a very common case.

Customer impact

Shorter, more readable code for a frequently used common case.

How found

Customer suggestion.

Regression

No. New API, and this is non-breaking for existing use of that API in RC1.

Testing

New tests added.

Risk

Low; non-breaking tweak to new API.

@smitpatel as discussed offline. Added various closure tests, funcletizer seems to be humming along with 100% efficiency.

AFAICT, switching from Expression to regular non-Expression doesn't seem to create particular difficulties for dynamic expression construction. Here's a sample:

await ctx.Blogs.ExecuteUpdateAsync(sp => sp
    .SetProperty(b => b.Name, "Hello"));

// Same thing dynamically:
var setPropertyMethodInfo = typeof(SetPropertyCalls<>).MakeGenericType(typeof(Blog))
    .GetMethods()
    .Single(m => m.Name == nameof(SetPropertyCalls<Blog>.SetProperty)
                 && m.GetParameters()[0].ParameterType.IsGenericType
                 && m.GetParameters()[0].ParameterType.GetGenericTypeDefinition() == typeof(Func<,>)
                 && !m.GetParameters()[1].ParameterType.IsGenericType)
    .MakeGenericMethod(typeof(string));

var setPropertyCallsParam = Expression.Parameter(typeof(SetPropertyCalls<Blog>), "sp");
var blogParam1 = Expression.Parameter(typeof(Blog), "b");

Expression<Func<SetPropertyCalls<Blog>, SetPropertyCalls<Blog>>> setProperties =
    Expression.Lambda<Func<SetPropertyCalls<Blog>, SetPropertyCalls<Blog>>>(
        Expression.Call(
            setPropertyCallsParam,
            setPropertyMethodInfo,
            Expression.Lambda<Func<Blog, string>>(Expression.Property(blogParam1, nameof(Blog.Name)), blogParam1),
            Expression.Constant("Hello")),
        setPropertyCallsParam);

await ctx.Blogs.ExecuteUpdateAsync(setProperties);

What do you think?

Closes #28968

@roji roji requested a review from smitpatel September 19, 2022 10:39
smitpatel
smitpatel reviewed Sep 19, 2022
View reviewed changes
test/EFCore.Relational.Specification.Tests/BulkUpdates/NorthwindBulkUpdatesTestBase.cs Outdated
rowsAffectedCount: 8,
(b, a) => Assert.All(a, c => Assert.Equal("Updated", c.ContactName)));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Update_Where_parameter_set_constant(bool async)
{
using var context = Fixture.CreateContext();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slipped in, removing.

@ajcvickers ajcvickers added this to the 7.0.0-rc2 milestone Sep 19, 2022
@ajcvickers
Copy link
Member

@roji Approved by Tactics. Please merge ASAP.

@roji
Allow specifying property value without lambda in ExecuteUpdate
497e155 
And make SetProperty accept a non-expression lambda directly

Closes dotnet#28968
@roji roji merged commit 4262584 into dotnet:release/7.0 Sep 20, 2022
@roji roji deleted the ExecuteUpdateThing branch September 20, 2022 06:50
ajcvickers pushed a commit that referenced this pull request Sep 20, 2022
@roji @ajcvickers
Allow specifying property value without lambda in ExecuteUpdate (#29139)
98f7c64 
And make SetProperty accept a non-expression lambda directly

Closes #28968
@ajcvickers ajcvickers removed this from the 7.0.0-rc2 milestone Sep 22, 2022
@ajcvickers ajcvickers mentioned this pull request Sep 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smitpatel smitpatel smitpatel approved these changes

Assignees
No one assigned
Labels
Servicing-approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@roji @ajcvickers @smitpatel